-
Notifications
You must be signed in to change notification settings - Fork 48
reexec: add "CommandContext" #193
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
5f9dbf9 to
a80dcff
Compare
|
Had to remove some bits from the test, because |
reexec/reexec_test.go
Outdated
| doc: "context timeout", | ||
| cmdAndArgs: []string{testReExec3}, | ||
| expOut: "Hello test-reexec3", | ||
| expError: "signal: killed", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, derp, and of course, windows has a different error;
=== RUN TestCommandContext/context_timeout
reexec_test.go:190: got "exit status 1", want "signal: killed"
f99a5f7 to
42b1ba7
Compare
|
Adds a CommandContext command, to provide the equivalent of exec.CommandContext. Signed-off-by: Sebastiaan van Stijn <[email protected]>
42b1ba7 to
7aa8514
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a CommandContext wrapper mirroring exec.CommandContext in the reexec package, enabling context-based cancellation for re-executed commands. Key changes:
- Added
CommandContextAPI inreexec.goand platform-specific implementations inreexec_other.goandreexec_linux.go. - Updated imports and registrations to support context-aware execution.
- Added
TestCommandContextinreexec_test.goto validate context cancellation, timeouts, and argument handling.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| reexec/reexec.go | Added public CommandContext function and imported context. |
| reexec/reexec_other.go | Introduced generic commandContext implementation. |
| reexec/reexec_linux.go | Added Linux-specific commandContext with Pdeathsig support. |
| reexec/reexec_test.go | New TestCommandContext covering basename, full path, args, cancel, and timeout cases. |
Comments suppressed due to low confidence (2)
reexec/reexec_test.go:139
- Missing import of "path/filepath", which is required for
filepath.Join. Please add"path/filepath"to the import block.
cmdAndArgs: []string{filepath.Join("something", testReExec2)},
reexec/reexec_other.go:19
- This
commandContextis duplicated inreexec_linux.go, leading to a redeclaration error on Linux builds. Add a build tag like// +build !linuxat the top of this file to restrict it to non-Linux platforms.
func commandContext(ctx context.Context, args ...string) *exec.Cmd {
Adds a CommandContext command, to provide the equivalent of exec.CommandContext.